feat: allow custom output via formatters #483
Conversation
jaulz
force-pushed
the
feat/add-templates
branch
from
087b13c to
12dd749
Compare
jaulz
changed the title
jaulz
changed the title
jaulz
changed the title
|
@bakerkretzmar I am not sure why the tests fail since I do not have the PHP version locally and there are no error messages in the output. With |
|
@jaulz I think the test failures were because of the This is a really cool idea... I'm going to think about it for a bit 😄 I like the flexibility it adds but I'm not sure how many people would actually use it. Can you give some more examples of what you're hoping to do? The example in the test you added just leaves out the Thanks! |
|
The idea that floats in my head is to also expose the payload schema to have local validations (as far as it's possible) and thus this would be the initial feature that would allow me to hook into the generation of the route definitions. I am not yet sure how it will look like and when I will work on it but it would be handy to be ready at any time :) |
|
Awesome idea and PR, I would for sure use it I just have one remark for the test unit though, instead of modifying the existing test, just copy it and then apply your modification to avoid 2 features overlapping in the same test |
|
Yeah, I could do that but I don't want to spend any time on it as long as @bakerkretzmar does not consider it to be merged. Just let me know in case it gets relevant 😊 |
There was a problem hiding this comment.
Okay looking at this again, I like it a lot, let's do it 😎 couple questions before I take a final pass and merge it in:
- I think I have a slight preference for resolving the formatters out of the application container, instead of listing classes in the config. So rather than
config()->get('ziggy.formatters.file', FileFormatter::class), something likeapp(FileFormatter::class, $ziggy). We could register our default ones in our service provider, and then to override them, users would bind their custom ones in their own service provider instead of publishing and filling in our config file. What do you think of that approach? Any drawbacks you can see? - I think it would be cool if we didn't need to call
->format()and the Script classes were stringable/htmlable. In theory they could then go directly in Blade templates and they would get cast to strings/HTML during render. This feels a tiny bit more flexible to me, and basically makes the formatters more into 'payload objects' that could do formatting or potentially other stuff too. I don't mind making this change myself. Maybe I'm over thinking this, curious to know what you think though!
|
Great to hear 😊 Regarding your points:
|
|
Nice, that looks good 👍 |
|
@jaulz thought about it some more and decided to keep your original implementation and get the classes from the config. Thanks for your patience! |
This PR will let developers define custom templates for the different parts that can be generated. I am not sure about the template syntax (i.e. ":key") but that's the Laravel translation syntax.This PR will let developers define custom formatters for the different parts that can be generated.